Move static_assert! into librustc_data_structures
#55805
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ rollup |
|
📌 Commit 87462067c91f8523689c8c6be516aa179c926f97 has been approved by |
bors
added
S-waiting-on-bors
|
@bors r- This is failing on |
bors
added
S-waiting-on-author
|
@rust-lang/wg-compiler-performance: Does anybody know why |
|
Preferred alignment of u8 and u16 on AArch64 is 4 bytes. |
|
@nagisa But that's not used for... pretty much anything, really. |
|
Is there a |
|
|
|
As discussed on IRC, another difference is that i128 is 128-bit aligned on Aarch64 and 64-bit aligned on x86_64. That’s the most likely root cause. |
nnethercote
force-pushed
the
mv-static_assert
branch
from
8746206
to
209f18a
Compare
|
I changed the assertion to be x86_64-only. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors try |
|
⌛ Trying commit 209f18aa8406ad3841700ac345f8a794f6ba4716 with merge 627a9b8857948721ee8a2eca7b33f36fced1a249... |
|
☀️ Test successful - status-travis |
src/libsyntax/ast.rs
Outdated
| // Note that the assertion is x86_64-only because the size is larger on | ||
| // AArch64. | ||
| #[cfg(target_arch = "x86_64")] | ||
| static_assert!(EXPR_IS_88_BYTES: std::mem::size_of::<Expr>() == 88); |
There was a problem hiding this comment.
It seems a bit incongruous that this assert is == whereas the Statement assert is <= -- I would think the same logic should apply to both.
There was a problem hiding this comment.
The Statement assert is used on all platforms, which is why it's looser -- the size on 32-bit is different to the size on 64-bit, etc.
The Expr assert is x86_64-only, because the size on AArch64 is apparently greater than 88, and I didn't want to give unnecessary leeway on x86_64, which is probably the most important platform. So I took the opportunity to make it stronger, using equality. The advantage of this is that if somebody does shrink Expr, say to 80 bytes, they must update the assertion accordingly, and then we can't accidentally backslide back to 88 bytes. That's what the "And if it gets smaller, this assertion should be updated so it stays at that smaller size" part of the comment is trying to communicate.
So, options:
- Accept the code as is.
- Expand comments on one or both assertions.
- Make the assertions more consistent, possibly by restricting the
Statementassertion to x86_64-only.
@Mark-Simulacrum: which would you prefer?
There was a problem hiding this comment.
I think it probably makes sense to gate the Statement assert to x86_64 as well and make it ==.
|
r=me with the asserts modified to match or explanation |
nnethercote
force-pushed
the
mv-static_assert
branch
from
209f18a
to
e01e0b0
Compare
|
@Mark-Simulacrum: I've made these changes:
|
|
@bors r+ |
|
📌 Commit e01e0b0 has been approved by |
bors
added
S-waiting-on-bors
…k-Simulacrum Move `static_assert!` into librustc_data_structures
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
| // Use the bool to access an array such that if the bool is false, the access | ||
| // is out-of-bounds. | ||
| #[allow(dead_code)] | ||
| static $name: () = [()][!$test as usize]; |
There was a problem hiding this comment.
(not really relevant to this PR) So I don't forget: this should have ($test: bool) to avoid accidentally allowing an integer expression (cc @oli-obk).
There was a problem hiding this comment.
Just came across this, and I'm curious to know where I can follow the status of having types as macro input specifiers. This would be nice for my const_assert! macro.
There was a problem hiding this comment.
I'm not sure what you mean. I am talking about writing [()][!($test: bool) as usize].
There was a problem hiding this comment.
Ah so this is just a matter of using type ascription anywhere. I was under the impression it was having bool instead of expr. It's still something I'll definitely be using when it stabilizes.
|
@bors r- (failed) |
bors
added
S-waiting-on-author
And make them x86_64-only so they can use `==` instead of `<=`.
nnethercote
force-pushed
the
mv-static_assert
branch
from
e01e0b0
to
2bd4d5b
Compare
|
I fixed the compile error, one of the @bors r=Mark-Simulacrum |
|
📌 Commit 2bd4d5b has been approved by |
bors
added
S-waiting-on-bors
…k-Simulacrum Move `static_assert!` into librustc_data_structures
Rollup of 20 pull requests Successful merges: - #55136 (Remove short doc where it starts with a codeblock) - #55711 (Format BtreeMap::range_mut example) - #55722 (impl_stable_hash_for: support enums and tuple structs with generic parameters) - #55754 (Avoid converting bytes to UTF-8 strings to print, just pass bytes to stdout/err) - #55804 (rustdoc: don't inline `pub use some_crate` unless directly asked to) - #55805 (Move `static_assert!` into librustc_data_structures) - #55837 (Make PhantomData #[structural_match]) - #55840 (Fix TLS errors when downloading stage0) - #55843 (add FromIterator<A> to Box<[A]>) - #55858 (Small fixes on code blocks in rustdoc) - #55863 (Fix a typo in std::panic) - #55870 (Fix typos.) - #55874 (string: Add documentation for `From` impls) - #55879 (save-analysis: Don't panic for macro-generated use globs) - #55882 (Reference count `crate_inherent_impls`s return value.) - #55888 (miri: for uniformity, also move memory_deallocated to AllocationExtra) - #55889 (global allocators: add a few comments) - #55896 (Document optimizations enabled by FusedIterator) - #55905 (Change `Lit::short_name` to `Lit::literal_name`.) - #55908 (Fix their/there grammar nit)
No description provided.